feat: implement lazy session restore feature with UI toggle - #93
feat: implement lazy session restore feature with UI toggle#93quarterpound wants to merge 4 commits into
Conversation
|
@gi11es any chance you can take a look at this? |
gi11es
left a comment
There was a problem hiding this comment.
Thanks — this is a well-constructed PR overall. A removed-behavior audit of the restructured restore path found the default (toggle-off) flow otherwise equivalent to the old eager code: tab order, suppressUnseen, onProcessExit wiring, selectedTabIndex clamping, and tmux name preservation all carry over. But verification confirmed one regression on the default path plus two UI issues, so requesting changes:
Confirmed — blocking
-
Orphaned process spawn on the default path (
startPendingTabsProgressively). The oldcreateTabsProgressivelybuilt tabs from saved state one-by-one, so a tab couldn't be closed before it existed. The new code creates allTabItems up front and holds live references inremaining, walking them at 0.05s cadence. Closing a not-yet-started tab (closeTabById → handleSurfaceClosedById) removes it fromworkspace.tabsbut never clearstab.pendingStart, so when its turn comes,startPendingShellIfNeededhappily callssurface.startShell— spawning an orphanedclaude --resume/codex/tmux process attached to no tab; itsonProcessExit → handleSurfaceClosedByIdis a no-op since the tab is gone. Fix options: clearpendingStartinhandleSurfaceClosedById, and/or havestartPendingShellIfNeededverify the tab is still present in some workspace before starting. -
Tab bar shows pending tabs as live. The sidebar learned
filled: !info.pending(hollow, non-pulsing dots), butHorizontalTabViewinTabBarViews.swiftconstructsBadgeShapeViewwithoutfilled:(defaultstrue) and still appliesisBadgeAnimated. Background tabs of the selected workspace are pending yet appear in the tab bar with solid — and potentially pulsing — dots, contradicting the sidebar one pane over. -
Never-started codex tabs flip to "thinking".
pollCodexBadgeStatesbuildscodexTargetsfrom all codex tabs with asessionId— deferred tabs keep their restored id — andparseCodexActivityInfohappily reads a stale on-disk.jsonlwhose last event istask_started, soapplyCodexBadgeStatessets.codexThinkingon a tab whose process was never spawned. Bounded (hollow dot, not persisted), but wrong: excludependingStart != niltabs fromcodexTargets.
Worth addressing
- Eager/lazy start paths have already drifted: the eager branch guards codex discovery with
tab.sessionId == nil || forkSession, the lazy copy drops|| forkSession(andPendingStartdoesn't captureforkSession). Latent today, but the duplication invites divergence — consider making the eager branch setpendingStartand immediately callstartPendingShellIfNeededso there's one start path. startPendingShellIfNeededtriggers a fullrebuildSidebar()on each first-open (defaultrefreshSidebar: truefromshowTab); cycling through N restored tabs costs N full sidebar rebuilds — a targeted badge refresh would do.- Minor:
PendingStartretainsenvVars/initialInputper never-opened tab indefinitely, andworkingDirectoryduplicatesworkspace.path.
One claim I explicitly checked and did NOT flag: closing a never-opened lazy terminal tab kills its tmux session — that matches the app's documented close-tab semantics, and closeWorkspace still takes the detach() branch thanks to the eager tmuxSessionName assignment. 👍
Also needs a rebase — the PR currently shows as conflicting with master.
… badges Addresses review feedback on lazy session restore (gi11es#93): - startPendingShellIfNeeded bails (and clears pendingStart) when the tab is no longer in any workspace, so the progressive-restore walk can't spawn an orphaned process for a tab closed before its deferred start ran; handleSurfaceClosedById also clears pendingStart on removal. - Tab bar renders deferred tabs' badge dots hollow and non-pulsing (filled: !pending, gated animation, "(not loaded)" tooltip), matching the sidebar. - Exclude deferred codex tabs from codex badge polling so a stale on-disk .jsonl can't drive a never-started tab to .codexThinking. - Capture forkSession in PendingStart and align the lazy codex-discovery guard with the eager path to prevent divergence. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… badges Addresses review feedback on lazy session restore (gi11es#93): - startPendingShellIfNeeded bails (and clears pendingStart) when the tab is no longer in any workspace, so the progressive-restore walk can't spawn an orphaned process for a tab closed before its deferred start ran; handleSurfaceClosedById also clears pendingStart on removal. - Tab bar renders deferred tabs' badge dots hollow and non-pulsing (filled: !pending, gated animation, "(not loaded)" tooltip), matching the sidebar. - Exclude deferred codex tabs from codex badge polling so a stale on-disk .jsonl can't drive a never-started tab to .codexThinking. - Capture forkSession in PendingStart and align the lazy codex-discovery guard with the eager path to prevent divergence. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Hello, thanks for the feedback. Please review |
|
@gi11es any chance you can take a look at this? |
On launch, Deckard restored every saved session by immediately spawning its process for every tab in every project. With many projects open, that's dozens of forkpty + agent processes fired within seconds of opening the app, plus a burst of TCC prompts and CPU churn for sessions you may never look at.
This PR makes session startup lazy-capable: tabs and their terminal views are still created up front (the sidebar and tab bar look identical), but the underlying shell/agent process can be deferred until a tab is first opened.
"Restore sessions lazily" toggle controls it, defaulting off so existing behavior is preserved.